-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add pip-compatible --group
flag to uv pip install
#11686
base: main
Are you sure you want to change the base?
Conversation
// Deduplicate in a stable way to get deterministic behaviour | ||
let deduped_paths = groups | ||
.iter() | ||
.map(|group| &group.path) | ||
.collect::<BTreeSet<_>>(); | ||
group_requirements = deduped_paths | ||
.into_iter() | ||
.map(|path| RequirementsSource::PyprojectToml(path.to_owned())) | ||
.collect::<Vec<_>>(); | ||
requirements = &group_requirements[..]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started doing a cleanup pass to compute a BTreeMap<PathBuf, Vec<GroupName>>
that would be used by subsequent steps but it was just more code and value threading for no real benefit over the current impl that just does a filter(group.path == path)
later.
FYI, pip support was merged. |
/// Ignore the package and it's dependencies, only install from the specified dependency group. | ||
/// | ||
/// May be provided multiple times. | ||
#[arg(long, group = "sources", conflicts_with_all = ["requirements", "package", "editable"])] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these conflicts present in the pip
implementation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(it doesn't really look like it?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought -r
was exclusive to our CLI (I didn't check the others though).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah -r
is exclusive to us, but I'd expect pip install -e . --group foo
and pip install anyio --group foo
to work unless they explicitly choose for it not to upstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Whether -r
should have the same semantics as these other options, I'm not sure)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One problem is that uv pip install -r pyproject.toml --extra foo
is the "correct" use of --extra
but uv pip install -r pyproject.toml --group foo
would be invalid? Even more confusing, if we allow it, uv pip install -r foo/pyproject.toml --group bar
would install bar
from ./pyproject.toml
instead of foo/pyproject.toml
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's part of why I opted to disallow it. The upstream pip semantics are really confusing to pair with anything else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's important that we're compatible with pip on whether --group
can be used with <package>
and --editable
though, which introduces all the same complexities.
This is a minimal redux of #10861 to be compatible with
uv pip
. I'm not 100% convinced the interaction with other sources is exactly what we want, but this is a good starting point.This implements the interface described in: pypa/pip#13065 (comment)
In that interface they add
--group <[path:]name>
topip install
,pip download
, andpip wheel
. Notably we do not defineuv pip download
anduv pip wheel
, so this PR only implements it foruv pip install
.It was previously proposed we should also support this flag for
uv pip compile
, which can be added trivially in this PR, but is currently not to minimize divergence.Fixes #8590
Current punts on #8969